AI Code Review

What Looks Good

• The fix is minimal and targeted, addressing a specific regression (CE0079) in microflow generation for IF statements with empty ELSE bodies and continuing THEN branches.
• The solution mirrors the existing empty-THEN handling pattern, maintaining consistency in the codebase.
• A new unit test (TestIfEmptyElseBodyWithContinuingThenEmitsFalseFlowToMerge) directly validates the fix and includes detailed comments explaining the regression scenario.
• No MDL syntax changes are introduced, so syntax design and full-stack consistency checks are not applicable.
• The change is scoped to a single logical fix (one commit) with no unrelated modifications.

Recommendation

Approve the PR. The fix resolves the reported issue with appropriate test coverage and follows existing code patterns. No changes are requested.

Automated review via OpenRouter (Nemotron Super 120B) — workflow source

CI stabilization note: merged #412 into this branch so it uses the same integration-CI baseline as the rest of the active PR set.

AI Code Review

Critical Issues

• Scope violation: The PR includes multiple unrelated changes beyond the stated fix for CE0079:
  1. TUI watcher debounce fix (cmd/mxcli/tui/watcher.go + test)
  2. Nanoflow examples file simplification (mdl-examples/doctype-tests/02b-nanoflow-examples.mdl)
  3. Roundtrip test updates (mdl/executor/roundtrip_doctype_test.go, mdl/executor/roundtrip_nanoflow_test.go)
     The PR description only mentions the IF builder fix. Per checklist: "Each PR is scoped to a single feature or concern — if the description needs 'and' between unrelated items, split it."

Moderate Issues

None

Minor Issues

None

What Looks Good

• The core fix for CE0079 in cmd_microflows_builder_control.go is correct and follows the existing empty-THEN pattern
• New unit test TestIfEmptyElseBodyWithContinuingThenEmitsFalseFlowToMerge properly validates the fix
• Changes are minimal and focused on the specific issue (when reviewing just the IF builder parts)

Recommendation

Please split this PR into three separate PRs:

1. IF builder fix (core change + new test): mdl/executor/cmd_microflows_builder_control.go and mdl/executor/cmd_microflows_builder_empty_else_merge_test.go
2. TUI watcher debounce fix: cmd/mxcli/tui/watcher.go and cmd/mxcli/tui/watcher_test.go
3. DocTest maintenance: Nanoflow examples simplification + roundtrip test updates (the nanoflow file change and related roundtrip test adjustments)

Only after splitting should the IF builder fix be approved. The other two changes are valid improvements but are unrelated to the stated bug fix.

Automated review via OpenRouter (Nemotron Super 120B) — workflow source

AI Code Review

Critical Issues

None found.

Moderate Issues

None found.

Minor Issues

• The nanoflow examples file (02b-nanoflow-examples.mdl) was completely rewritten and simplified. While this doesn't appear to be incorrect, it's a significant change that isn't directly related to the main fix (CE0079). However, since it's a doctest file and the simplification maintains coverage of nanoflow features, this is acceptable.

What Looks Good

Main Fix (CE0079)

• Correctly identifies root cause: Missing "false" flow in IF statements with empty ELSE bodies causes Studio Pro validation error CE0079
• Mirrors existing pattern: Solution follows the same approach as empty-THEN handling already present in the codebase
• Well-tested: New unit test TestIfEmptyElseBodyWithContinuingThenEmitsFalseFlowToMerge specifically verifies the fix
• Minimal and targeted: Only adds the necessary flow without disturbing existing logic

TUI Watcher Race Condition Fix

• Proper concurrency handling: Uses atomic.Uint64 for debounce sequence instead of plain integer
• Improved test reliability: Removed unreliable time.Sleep from test and added explanatory comment
• Thread-safe: Prevents potential race conditions in the watcher debounce mechanism

Java Action Return Type Handling

• Correct void return type processing: Added buildJavaActionReturnType and isVoidReturnType helper functions
• Test coverage: New test TestJavaAction_ExplicitVoidReturnType validates explicit void return types
• Logical placement: Helper functions fit naturally in the visitor Java action file

Test Coverage

• New test file for the IF/ELSE fix (cmd_microflows_builder_empty_else_merge_test.go)
• Existing tests updated appropriately (watcher, roundtrip, visitor tests)
• No use of time.Sleep for synchronization (in fact, one was removed)
• Doctype test files still provide meaningful examples after simplification

Recommendation

Approve. The PR correctly fixes the CE0079 issue with a well-tested solution that mirrors existing patterns. Additional improvements to the TUI watcher (race condition fix) and Java action return type handling are valuable robustness enhancements. All changes are focused, properly tested, and maintain code quality standards. The nanoflow examples simplification, while somewhat tangential, doesn't harm functionality and may improve maintainability.

Automated review via OpenRouter (Nemotron Super 120B) — workflow source

Review: fix: emit merge flow for empty ELSE bodies (CE0079)

Two blockers before this can merge.

Blocker 1 — test doesn't exercise the new code

The test creates IfStmt with no HasElse and empty ElseBody:

fb.addIfStatement(&ast.IfStmt{
    Condition: ...,
    ThenBody: []ast.MicroflowStatement{&ast.LogStmt{...}},
    // ElseBody deliberately empty
})

With the current hasElseBody := len(s.ElseBody) > 0 (line 34 of cmd_microflows_builder_control.go), this evaluates to false, so execution takes the IF-WITHOUT-ELSE branch at line 304 — not the new code path. The IF-WITHOUT-ELSE branch already emits newHorizontalFlowWithCase(splitID, mergeID, "false") when needMerge = true. The test was already passing before this PR.

The new else block lives inside if hasElseBody { ... if !elseReturns && needMerge { ... } }. It can only be reached when hasElseBody = true and lastElseID = "", which requires an explicitly empty ELSE body that the visitor records with HasElse: true — the HasElse field added by PR #366.

The test comment acknowledges this ("see below") but there is no second test in the PR.

A test that exercises the new code must set both HasElse: true and an empty ElseBody in the AST, and also requires the hasElseBody calculation to be updated to s.HasElse || len(s.ElseBody) > 0 (see Blocker 2).

Blocker 2 — hidden dependency on PR #366 (HasElse + updated hasElseBody)

The new else block is dead code in the current codebase. Until PR #366 changes hasElseBody to s.HasElse || len(s.ElseBody) > 0, no MDL input can produce hasElseBody = true with an empty ELSE body. The grammar produces s.ElseBody = nil or [] for if cond then ... else end if, so len(s.ElseBody) = 0 → hasElseBody = false → IF-WITHOUT-ELSE path.

This PR should either:

• Be merged after PR fix: preserve custom error handler continuations #366, with the test using HasElse: true directly in the AST
• Include PR fix: preserve custom error handler continuations #366's hasElseBody fix as part of this PR
• At minimum document the dependency explicitly in the PR description

Minor — bundled unrelated changes

Watcher debounce (sync/atomic), Java action RETURNS Void handling, nanoflow doctype cleanup, enum parameter roundtrip test — none of these relate to the CE0079 fix. Same pattern seen in PRs #366, #370. These belong in separate PRs.

The fix itself is correct

For the scenario it targets (empty ELSE with hasElseBody = true), adding newDownwardFlowWithCase(splitID, mergeID, "false") inside if !elseReturns && needMerge mirrors the existing empty-THEN logic exactly and will produce the correct BSON for Studio Pro. The fix just can't be reached or tested until the HasElse + hasElseBody prerequisite from PR #366 is in place.

Addressed the review blocker:

• stacked this branch explicitly on fix: preserve custom error handler continuations #366, because the fix needs IfStmt.HasElse to distinguish omitted ELSE from explicitly empty ELSE;
• updated the regression test to set HasElse: true, so it now exercises the explicit empty-ELSE path;
• reran go test ./mdl/executor -run TestIfEmptyElseBodyWithContinuingThenEmitsFalseFlowToMerge -v, make build, make test, and make lint-go.

Follow-up: after #366 landed on main, I rebuilt this branch directly on current origin/main and force-pushed a clean single-commit PR. The diff is now limited to the empty-ELSE builder fix plus its regression test; the test still sets HasElse: true to exercise the explicit empty-ELSE path.